feat(runtime): MCP progress projector and warm-runtime proofs (#96) - #175
Conversation
Born on the Effect render-event stream: emit notifications/progress only when the caller supplied a token, return one CallToolResult, and fail closed on epoch mismatch, restart, and a missing runtime.
🦋 Changeset detectedLatest commit: 6c8078d The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c8078d36a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ' worker.on(\'message\', (message) => {', | ||
| ' const request = pending.get(message.id);', | ||
| ' if (request === undefined) return;', | ||
| " if (message.type === 'progress') { void request.progress?.report(message.update); return; }", |
There was a problem hiding this comment.
Await worker progress forwarding
When a generated tool reports more than the session's buffered progress limit (10,000 by default) before returning, request.progress.report() rejects with event-count-exceeded, but this discarded promise becomes an unhandled rejection in the MCP server process. Under the supported Node versions, that can terminate the entire stdio server instead of failing only the offending tool call; forwarding needs to be awaited/serialized (with worker acknowledgement for backpressure) or its rejection must be routed into the pending request.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in #210 (merged as 1c36813): the generated shell no longer voids progress forwarding — rejections (and sync throws) from entry.progress.report route into the request failure path via entry.fail, so a progress-cap rejection fails the request instead of becoming an unhandled worker rejection. Regression test in entry-shell.test.ts runs the generated factory with a rejecting reporter and asserts the request fails with no unhandled rejection.
…endered workers, canonical result output (#210) * fix(cli): worker-name collisions, negative positionals, fail-closed rendered workers, canonical result output Post-merge review findings from #186, #175, and #178: reserve generated *-flight.mjs worker destinations during entry planning so a conventional script colliding with a rendered worker is a build-time AB5000 diagnostic instead of a silent overwrite; treat a lone -<digit> token as a positional when the next positional expects a number (keeping -- escape and unknown single-dash rejection intact); fail all pending rendered requests closed when the worker exits with any code, including 0; route rejected progress reports into the request failure path instead of voiding them; serialize plain CLI results with the canonical stableJson writer (undefined emits null); and validate the exitCode result mapping before writing stdout so an invalid mapping cannot emit a success line first. The inlined cli-entry runtime now ignores its owning source root during builds because it pulls sibling core imports. * fix(build): anchor the runtime ignored root to the module's marker ancestor Review follow-up on #210: runtimeIgnoredRoot matched any /dist/ segment in the path, so a checkout living under a dist directory resolved the ignored root to the wrong ancestor and silently ignored the whole checkout. The root is now the parent of the nearest src or dist ancestor of the runtime module itself, and an unmarked path is a loud error.
Summary
Wave 4 Lane D / #96 stage 4: standards-compatible MCP projector over the Effect render-event stream, plus warm-runtime fail-closed proofs.
projectMcpRenderStreamemitsnotifications/progressonly when the caller supplied a progress token; maps monotonically increasing numericprogress+ optionaltotal+ short message (cap 200); buffers shell/replace internally and never encodes partial content in progress messages; returns oneCallToolResultwith supported blocks and object-valued structured content.McpProjectionError— no silent drops.Agent.Contextprojects to text (MCP has no additional-context block).createWarmFlightHost+ generated-entry worker: one process-lifetime provider across consecutivetools/calls; artifact-epoch mismatch, worker restart, and a missing runtime fail closed with typedAgentRuntimeError(MCP surfaces those asisErrorresults, never fabricated success).CreateTaskResult+tasks/get+tasks/result+tasks/cancel) is not in this stage.Projector contract (what emits when)
_meta.progressTokennotifications/progress; finalCallToolResultonlycompletednotifications/progresswithprogress, optionaltotal, shortenedmessageshell/replacecompleteCallToolResult; progress nodes omitted from content; objectdocument.value→structuredContentvaluestructuredContentomitted (not wrapped)McpProjectionError(unsupported-rich-content)notifications/cancelledAbortSignal(SDK already abortscontext.mcpReq.signal)Warm-runtime proofs
warmthcalls: hits 1 then 2, sameinstanceIdandpid.AgentRuntimeError(artifact-epoch-mismatch), host not executed.process.exit(1)then nextwarmth:isError/ typed restart-or-unavailable, neverhits: 2.runtime-unavailable.Test plan
pnpm lint(0 errors)pnpm typecheckpackages/agent-bundle/tests/entry-shell.test.ts(11 passed)generated-route-server.test.tsintegration (5 passed, retry:2)pnpm eval:spot(passed)Stage 5/6 still needs
CreateTaskResult,tasks/get,tasks/result,tasks/cancel)lowerMcpResultonce generated + handwritten MCP entries all use the stream projectorHelped / hurt
Stream+interruptWhenAborted; progress is status-only; fix(runtime): review follow-ups — traversal bounds, demand-bounded progress, failure conversion #172 already demand-bounds live progress.SdkErrorand tool failures asCallToolResult.isError; Flight workers inherit stdout unless piped.